Skip to content

feat(review): add a performance-regression instruction to the AI review system prompt#2636

Merged
JSONbored merged 2 commits into
mainfrom
feat/ai-review-performance-regression-instruction
Jul 2, 2026
Merged

feat(review): add a performance-regression instruction to the AI review system prompt#2636
JSONbored merged 2 commits into
mainfrom
feat/ai-review-performance-regression-instruction

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • REVIEW_SYSTEM_PROMPT (src/services/ai-review.ts) exhaustively defined blocker criteria — bug/security-hole/data-loss/build-break/API-contract-break — but never mentioned algorithmic complexity, N+1 queries, or unbounded loop/fanout. REES's ~19 analyzers don't target the PR's own new code's runtime performance either (the closed feat(rees): add performance guardrails and regression telemetry #1814 is about REES's OWN service budgets, a different thing). A genuinely bad performance regression (a DB query moved inside a loop) was structurally invisible to the reviewer.
  • Adds a performance-regression category to the blockers taxonomy bullet, and a new PERFORMANCE SEVERITY discipline line matching the existing "don't nitpick, flag what matters" tone — a concrete, visible regression (query/network call moved inside a loop, an unbounded loop/fanout) is a blocker; a style/micro-optimization preference stays a nit.
  • Pure prompt-engineering change: no new service, no new cost, no new analyzer.

Closes #2559.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

  • git diff --check
  • npm run actionlint (via npm run test:ci)
  • npm run typecheck
  • npm run test:coverage locally — a test asserts the real system prompt sent to the model includes the new blocker category, the PERFORMANCE SEVERITY line, and the severity-discipline phrasing; mutation-tested (reverted the addition, confirmed the test fails).
  • npm run test:workers (via npm run test:ci)
  • npm run build:mcp (via npm run test:ci)
  • npm run test:mcp-pack (via npm run test:ci)
  • npm run ui:openapi:check (via npm run test:ci) — no OpenAPI-visible change
  • npm run ui:lint (via npm run test:ci)
  • npm run ui:typecheck (via npm run test:ci)
  • npm run ui:build (via npm run test:ci)
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A, no auth/session/CORS surface touched.
  • API/OpenAPI/MCP behavior is updated and tested where needed. — N/A, no API-visible change (internal prompt text only).
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A, no UI change.
  • Visible UI changes include a UI Evidence section below with screenshots. — N/A, no UI change.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. — N/A.

Notes

…ew system prompt

REVIEW_SYSTEM_PROMPT exhaustively defined blocker criteria
(bug/security/data-loss/build-break/API-contract-break) but never
mentioned algorithmic complexity, N+1 queries, or unbounded loop/fanout —
a genuinely bad performance change (e.g. a DB query moved inside a loop)
was structurally invisible to the reviewer, not merely under-prioritized.

Adds a performance-regression category to the blockers taxonomy plus a
PERFORMANCE SEVERITY discipline line matching the existing "don't nitpick,
flag what matters" tone: a concrete, visible regression is a blocker, a
style/micro-optimization preference stays a nit.

Closes #2559.
@dosubot dosubot Bot added the size:XS label Jul 2, 2026
@loopover-orb

loopover-orb Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-02 22:25:26 UTC

2 files · 1 AI reviewer · no blockers · readiness 80/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The change cleanly extends the AI-review system prompt so visible algorithmic or fanout regressions can be classified as blockers while preserving the existing severity boundary for micro-optimizations. The unit test exercises the real review path, captures the emitted system prompt, and verifies the new category and severity guidance are actually sent to the model. I do not see a correctness, wiring, or contract break in this focused prompt-engineering diff.

Nits — 4 non-blocking
  • nit: test/unit/ai-review.test.ts:1788 repeats a full AI-review invocation just to inspect the system prompt; a shared helper for extracting the system message would make future prompt assertions less noisy.
  • test/unit/ai-review.test.ts:1788: consider extracting the run/mock/env setup into a small local helper alongside the other prompt-instruction tests so additions to REVIEW_SYSTEM_PROMPT stay easy to review.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #1814, #2559
Related work ⚠️ 2 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (size label size:XS; 2 linked issues).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 64 registered-repo PR(s), 55 merged, 522 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 64 PR(s), 522 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Review top overlaps.
  • Add a concise scope and risk note.
  • No action.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added gittensor gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. labels Jul 2, 2026
@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.05%. Comparing base (bcc19ea) to head (6cc9a99).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2636   +/-   ##
=======================================
  Coverage   96.05%   96.05%           
=======================================
  Files         234      234           
  Lines       26280    26280           
  Branches     9531     9531           
=======================================
  Hits        25244    25244           
  Misses        425      425           
  Partials      611      611           
Files with missing lines Coverage Δ
src/services/ai-review.ts 95.17% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…nce-regression prompt test

Gate review nit: the test only asserted a few substrings, so it would
still pass if the unbounded loop/fanout example were accidentally removed
from REVIEW_SYSTEM_PROMPT.
@JSONbored JSONbored self-assigned this Jul 2, 2026
@JSONbored
JSONbored merged commit 431dfbd into main Jul 2, 2026
12 checks passed
@JSONbored
JSONbored deleted the feat/ai-review-performance-regression-instruction branch July 2, 2026 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Development

Successfully merging this pull request may close these issues.

feat(review): add a performance-regression instruction to the AI review system prompt feat(rees): add performance guardrails and regression telemetry

1 participant